Fix fmod post-condition check for negative remainders - #771
Conversation
The check compared signed values instead of magnitudes, so it fired on every negative remainder, git_issue_604.cpp now covers non-zero remainders too
dc6048c to
16b57d0
Compare
|
Hi @0x74h51N I just approved your workflow run. In this repo first-time contributors need workflow approval run. I see you also added relevant tests to this issue resolution. Thank you for those as well. Let's see how CI runs. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #771 +/- ##
=========================================
+ Coverage 96.2% 96.2% +0.1%
=========================================
Files 302 302
Lines 29478 29512 +34
=========================================
+ Hits 28344 28378 +34
Misses 1134 1134
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
Hi @0x74h51N the CI run went green. There are two missed coverage lines. But we can fill in more tests later. This is better than the previous state. So I am going to merge to develop now. Thank you very much for looking into this and improving the logic. Cc: @jzmaddock and @mborland and @cosurgi |
2 participants
Fixes #764
The post-condition added in #605 tests |r| >= |b|, but does it with a signed compare against an operand matched to the remainder's sign. That equivalence only holds while both values are positive; once the remainder is negative the ordering reverses and every valid remainder satisfies it, so the correction fires on every call and adds one |b|. What comes back is the Euclidean remainder instead of the truncated one.
The comparison is now read by magnitude. The branch structure and the subtraction directions are unchanged.
git_issue_604.cpp covered all four sign combinations, but only with a zero remainder, where both conventions agree, so it could not separate them. It now also covers non-zero remainders and |a| < |b|.
This keeps fmod on std::fmod semantics, which is what float128 and mpfr_float still return.